ElectraForMultipleChoice by patil-suraj · Pull Request #4954 · huggingface/transformers

patil-suraj · 2020-06-12T07:48:21Z

This PR add ElectraForMultipleChoice. One of the missing models in this project.

Since, for multiple choice pooled outputs are needed, added ElectraPooler class.

codecov · 2020-06-12T07:55:46Z

Codecov Report

Merging #4954 into master will increase coverage by 0.08%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4954      +/-   ##
==========================================
+ Coverage   77.24%   77.33%   +0.08%     
==========================================
  Files         133      133              
  Lines       22134    22166      +32     
==========================================
+ Hits        17097    17141      +44     
+ Misses       5037     5025      -12

Impacted Files	Coverage Δ
src/transformers/__init__.py	`99.16% <ø> (ø)`
src/transformers/modeling_auto.py	`70.76% <ø> (ø)`
src/transformers/configuration_electra.py	`100.00% <100.00%> (ø)`
src/transformers/modeling_electra.py	`80.12% <100.00%> (+1.95%)`	⬆️
src/transformers/modeling_utils.py	`91.06% <0.00%> (+0.12%)`	⬆️
src/transformers/trainer.py	`39.62% <0.00%> (+0.23%)`	⬆️
src/transformers/modeling_tf_utils.py	`87.57% <0.00%> (+1.55%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update efeb75b...9cb71f9. Read the comment docs.

sgugger

Thanks for your PR! I think there is a way to make it more consistent with the other models for multiple choice and avoid introducing a new class, would you mind looking into it?

sgugger · 2020-06-12T11:21:32Z

src/transformers/modeling_electra.py

        return outputs  # (loss), start_logits, end_logits, (hidden_states), (attentions)
+
+
+class ElectraPooler(nn.Module):


Instead of introducing a new class, I'd rather use the existing SequenceSummary which can do the pool + dropout (see the XLNetForMultipleChoice for an example. It will require to add a few things to the config to make it work (see XLNetConfig for an example).

I think there is a way to make it more consistent with the other models for multiple choice and avoid introducing a new class, would you mind looking into it?

Sure, I wasn't aware of SequenceSummary. I'll try to use it instead of Pooler

But couldn't you just use the ElectraClassificationHead for that pooling 🤔:

transformers/src/transformers/modeling_electra.py

Lines 346 to 362 in 86578bb

class ElectraClassificationHead(nn.Module):

"""Head for sentence-level classification tasks."""

def __init__(self, config):

super().__init__()

self.dense = nn.Linear(config.hidden_size, config.hidden_size)

self.dropout = nn.Dropout(config.hidden_dropout_prob)

self.out_proj = nn.Linear(config.hidden_size, config.num_labels)

def forward(self, features, **kwargs):

x = features[:, 0, :] # take <s> token (equiv. to [CLS])

x = self.dropout(x)

x = self.dense(x)

x = get_activation("gelu")(x) # although BERT uses tanh here, it seems Electra authors used gelu here

x = self.dropout(x)

x = self.out_proj(x)

return x

Sadly no, because it adds a linear layer to num_labels

sgugger

LGTM, @LysandreJik it's good for your review.

LysandreJik

LGTM, thanks @patil-suraj

patil-suraj · 2020-06-18T09:50:35Z

hi @LysandreJik could you tell me why these tests are failing now ? Thanks.

sgugger · 2020-06-18T12:05:31Z

Looks like @LysandreJik deleted a function by mistake during the merge (the create_and_check_electra_for_multiple_choice), could you add it back?

LysandreJik · 2020-06-18T23:01:14Z

My bad, sorry about that. Thanks for the fix!

patil-suraj added 4 commits June 12, 2020 13:06

add ElectraForMultipleChoice

62a10e3

add test_for_multiple_choice

7dcc5c0

add ElectraForMultipleChoice in auto model

54c9674

add ElectraForMultipleChoice in all_model_classes

8b3c81d

sgugger suggested changes Jun 12, 2020

View reviewed changes

patil-suraj added 2 commits June 12, 2020 17:52

add SequenceSummary related parameters

b01ce7c

get rid pooler, use SequenceSummary instead

c2b9d68

sgugger approved these changes Jun 12, 2020

View reviewed changes

sgugger requested a review from LysandreJik June 12, 2020 18:23

LysandreJik approved these changes Jun 17, 2020

View reviewed changes

Merge branch 'master' into electra-for-multiple-choice

c7ec642

add electra multiple choice test

9cb71f9

sgugger merged commit ca2d0f9 into huggingface:master Jun 18, 2020

patil-suraj deleted the electra-for-multiple-choice branch June 19, 2020 05:14

bogdankostic mentioned this pull request Oct 21, 2020

Fix random behavior when loading ELECTRA models deepset-ai/FARM#599

Merged

ydshieh mentioned this pull request Feb 14, 2022

Fix TFSequenceSummary's activation #15643

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ElectraForMultipleChoice#4954

ElectraForMultipleChoice#4954
sgugger merged 8 commits intohuggingface:masterfrom
patil-suraj:electra-for-multiple-choice

patil-suraj commented Jun 12, 2020

Uh oh!

codecov bot commented Jun 12, 2020 •

edited

Loading

Uh oh!

sgugger left a comment

Uh oh!

sgugger Jun 12, 2020

Uh oh!

patil-suraj Jun 12, 2020

Uh oh!

stefan-it Jun 12, 2020

Uh oh!

sgugger Jun 12, 2020

Uh oh!

sgugger left a comment •

edited

Loading

Uh oh!

LysandreJik left a comment

Uh oh!

patil-suraj commented Jun 18, 2020

Uh oh!

sgugger commented Jun 18, 2020

Uh oh!

LysandreJik commented Jun 18, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		return outputs # (loss), start_logits, end_logits, (hidden_states), (attentions)


		class ElectraPooler(nn.Module):

	class ElectraClassificationHead(nn.Module):
	"""Head for sentence-level classification tasks."""

	def __init__(self, config):
	super().__init__()
	self.dense = nn.Linear(config.hidden_size, config.hidden_size)
	self.dropout = nn.Dropout(config.hidden_dropout_prob)
	self.out_proj = nn.Linear(config.hidden_size, config.num_labels)

	def forward(self, features, **kwargs):
	x = features[:, 0, :] # take <s> token (equiv. to [CLS])
	x = self.dropout(x)
	x = self.dense(x)
	x = get_activation("gelu")(x) # although BERT uses tanh here, it seems Electra authors used gelu here
	x = self.dropout(x)
	x = self.out_proj(x)
	return x

Conversation

patil-suraj commented Jun 12, 2020

Uh oh!

codecov bot commented Jun 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sgugger left a comment

Choose a reason for hiding this comment

Uh oh!

sgugger Jun 12, 2020

Choose a reason for hiding this comment

Uh oh!

patil-suraj Jun 12, 2020

Choose a reason for hiding this comment

Uh oh!

stefan-it Jun 12, 2020

Choose a reason for hiding this comment

Uh oh!

sgugger Jun 12, 2020

Choose a reason for hiding this comment

Uh oh!

sgugger left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LysandreJik left a comment

Choose a reason for hiding this comment

Uh oh!

patil-suraj commented Jun 18, 2020

Uh oh!

sgugger commented Jun 18, 2020

Uh oh!

LysandreJik commented Jun 18, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Jun 12, 2020 •

edited

Loading

sgugger left a comment •

edited

Loading